perf(runtime): the receiver own-key probe stops scanning the keys array element by element - #9190

sequenceDiagram
  participant ReflectSet
  participant obj_value_has_own_key
  participant keys_find_slot_by_key_ptr
  participant Receiver
  ReflectSet->>obj_value_has_own_key: probe receiver own key
  obj_value_has_own_key->>keys_find_slot_by_key_ptr: search backing key storage
  keys_find_slot_by_key_ptr-->>obj_value_has_own_key: return key slot result
  obj_value_has_own_key-->>ReflectSet: return own-key status
  ReflectSet->>Receiver: apply receiver property update

• Fix all pre-merge checks with AI

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands.}

Rebased onto 84185b5656 and fixed the two lint failures. No change to the runtime diff itself.

• changelog fragment — added changelog.d/9190-set-receiver-own-key-index.md.
• file-size cap — the new pin took crates/perry-runtime/src/object/tests.rs from 1,991 to 2,088 lines. Moved it to its own object/set_receiver_tests.rs (the same split tombstone_tests.rs already uses), which puts tests.rs back at 1,991 and the new module at 102. ./scripts/check_file_size.sh is clean, and cargo test -p perry-runtime --lib object:: is 256 passed / 0 failed locally.

Two other reds on the previous run, neither of which I believe is this diff — recording what I checked rather than asserting it:

e2e-scoped is a main breakage, not mine: ci_e2e_scope --self-test fails because crates/perry-codegen/tests/string_array_length_9160.rs landed with #9160 without a _CODEGEN_SUITES entry, and that map is complete-by-construction. It is red on every core PR right now. Fixed separately in #9193.

cargo-test aborted with a stack overflow, not an assertion — commands::compile::collect_modules::tests::statically_reachable_trusted_js_package_is_aot_compiled_without_route_entry, a test from #8529 in the perry compiler crate, while this diff is runtime-only. Reads like a debug-build recursion-depth flake.

gc-stress reported PASS=449 UNVER=124 XFAIL=0 FAIL=1: test_gap_repsel_pshape_tower_delete, output-mismatch, in exactly one of the seven PR arms (force_verify) — and that cell reports evacuated=0, so the arm's distinguishing behaviour did not actually fire. That test is about delete compacting inline slots behind a cloned keys array, which is adjacent enough to this diff that I am not willing to call it unrelated on a reading of the code. I am reproducing it locally on this branch under that exact arm; if it reproduces I will root-cause it before asking anyone to merge this.

Validated on claude-code itself: this chain is 10.5% of cc --help.

A fresh symbolized profile of current main (84185b5656), 12 reps of claude --help, PERRY_DEBUG_SYMBOLS=1:

 4.54%  js_array_get_f64
 3.15%  perry_runtime::string::compare::js_string_key_matches
 1.83%  RuntimeHandle::across_mut::<ArrayHeader, JSValue,
              object::reflect_support::obj_value_has_own_key::{closure#3}>
 0.96%  perry_runtime::object::reflect_support::obj_value_has_own_key

js_array_get_f64 is the largest single symbol in the entire profile, and the three beneath it are the rest of the same loop: the per-iteration handle round-trip (the closure symbol names obj_value_has_own_key outright), the per-element compare, and the function itself. This PR removes all four — the element read, the re-rooting, and the compare are replaced by one keys_find_slot_by_key_ptr probe.

Context, same box, five interleaved reps:

A note on how this was validated, because the obvious benchmark was wrong. Session 88 measured plain-object stores at 17–29× node and offered the curve as this PR's validation. Counting rather than timing killed it: obj_value_has_own_key and js_array_get_f64 appear nowhere on the plain-store path — a plain literal receiver never leaves the direct-store lane, and one Object.defineProperty is what puts it on this one. That is why the pin in this PR counts js_array_get_f64 entries (test_element_accessor_calls) instead of timing anything, and why the number quoted in the commit message is 163,200 element reads → 0 rather than a duration. The cc profile above is the timing evidence, from the workload that actually matters.

Merged.

The counter is the part I want to single out. Asserting 163,200 → 0 calls into js_array_get_f64 pins the property — that the walk no longer reaches for the element accessor — where a timing assertion would have pinned the machine it was measured on and gone flaky the first time CI was busy. That's the right instrument for this change, and it's the difference between a test that can still fail for the right reason in a year and one that gets quarantined.

The reasoning behind the change holds up too: the element accessor runs forward resolution, the Map/Set/typed-array/buffer registry probes, the descriptor gate and hole translation, and the own-key walk needs none of it — it wants a raw slot read and knows the storage is dense.

Validation: test_gap_9180_receiver_set_own_key_scan.ts matches node 26.5.1 byte-for-byte across all 53 assertions, including the ones most likely to catch a too-aggressive direct read — Reflect.set with a distinct receiver, non-writable and accessor properties on the target, a non-extensible receiver, the Proxy trap log ordering, index-vs-string key aliasing ("2" vs "02"), prototype shadowing, and delete-then-readd. perry-runtime 2853 passed / 0 failed at RUST_TEST_THREADS=1; all 60 lint gates green.

My own differential probe over the same area (300+ named keys, numeric/string key aliasing, negative and fractional keys, non-enumerable and accessor descriptors, symbol keys, getOwnPropertyNames, for…in order under prototype shadowing, Object.freeze, and a 400-key index-like receiver) matches node too, with one exception that is not yours: arr.foo = "bar" on an array is silently dropped — the read returns undefined and the key never appears in Object.keys. I confirmed it pre-existing by reverting your four runtime files to main and rebuilding the runtime and stdlib static wrappers: byte-identical output, both differing from node in exactly those two lines. Filed as #9201; it's a silent failure on a pattern real packages use (arrays with named metadata fields), so it seemed worth its own issue rather than a footnote.

The two across_mut::<ArrayHeader, _>(|| ()) / across_const::<StringHeader, _>(|| ()) sites are the vacuous form of that helper — with a no-op closure nothing can move, so the re-resolution is a verbose deref rather than a safety measure. Not blocking and not a mark against this PR: the pattern is endemic on main (~16 existing sites), which is why I filed #9152 about it separately rather than raising it per-PR.

Added the changelog.d/ fragment, which the branch was missing.